Description

Video is recorded into a worker-level directory while a single worker-scoped Page stays open across
all tests, producing one long video finalized at context close instead of per-test videos. This
makes it difficult to correlate a specific failure to the right video segment and won’t integrate
with Playwright’s per-test video artifacts.

Code

e2e-tests/playwright/support/shared-page.ts[R42-74]

+      const context = await browser.newContext({
+        recordVideo: {
+          dir: videoDir,
+          size: { width: 1280, height: 720 },
+        },
+      });
+
+      await context.tracing.start({
+        screenshots: true,
+        snapshots: true,
+        sources: false,
+      });
+
+      await use(context);
+
+      await context.tracing.stop();
+      await context.close();
+
+      // Retain-on-failure: delete video files when all tests passed
+      if (!workerHadFailure && fs.existsSync(videoDir)) {
+        fs.rmSync(videoDir, { recursive: true, force: true });
+      }
+    },
+    { scope: "worker" },
+  ],
+
+  sharedPage: [
+    async ({ sharedContext }, use) => {
+      const page = await sharedContext.newPage();
+      await use(page);
+    },
+    { scope: "worker" },
+  ],

Relevance

⭐⭐ Medium

Team values artifacts (traces/videos), but worker-scoped context/page reuse seems intentional;
per-test video may be acceptable.

PR-#4248
PR-#3893
PR-#4437

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The fixture enables recordVideo on the worker-scoped context and creates exactly one worker-scoped
page that is not closed per test. As a result, video output is produced for the lifetime of the
worker context, not aligned to individual tests. The repo’s Playwright config also documents that
the built-in per-test video behavior applies only when using the built-in { page } fixture.

e2e-tests/playwright/support/shared-page.ts[34-74]
e2e-tests/playwright.config.ts[69-84]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The current implementation records one continuous per-worker video because the page is worker-scoped and never closed between tests. This reduces the usefulness of videos for debugging individual failures.

### Issue Context
Playwright’s config is set up for per-test `video: { mode: 'retain-on-failure' }`, but that automatic behavior applies to the built-in `{ page }` fixture.

### Fix Focus Areas
- Consider making `sharedPage` test-scoped (new page per test) while keeping a shared authenticated context via `storageState`.
- Or manually create/close a per-test page and attach its video to `testInfo` (closing the page per test is typically required to finalize video files).

- e2e-tests/playwright/support/shared-page.ts[34-74]
- e2e-tests/playwright.config.ts[69-84]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

Because sharedContext/sharedPage are worker-scoped, a retried test will run in the same
BrowserContext/Page state as the failed attempt, undermining Playwright retry isolation. With
retries enabled in the global Playwright config, this can cause false passes/fails and hard-to-debug
flakiness.

Code

e2e-tests/playwright/support/shared-page.ts[R65-74]

+    { scope: "worker" },
+  ],
+
+  sharedPage: [
+    async ({ sharedContext }, use) => {
+      const page = await sharedContext.newPage();
+      await use(page);
+    },
+    { scope: "worker" },
+  ],

Relevance

⭐ Low

Repo already reuses context/page via beforeAll in serial suites; retries already existed with same
dirty-state issue.

PR-#3893
PR-#3232
PR-#3860

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The sharedContext and sharedPage fixtures are explicitly worker-scoped, so they are created once per
worker and reused for all tests executed by that worker. The repo configuration enables retries on
CI, which means failed tests will be re-run without recreating those worker-scoped fixtures, so
browser state (cookies, localStorage, current URL, listeners, etc.) carries into the retry attempt.

e2e-tests/playwright/support/shared-page.ts[34-75]
e2e-tests/playwright.config.ts[53-61]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`sharedContext`/`sharedPage` are `scope: "worker"`, so Playwright retries re-run in the same context/page state as the failed attempt. This breaks retry isolation and can hide/mask flaky failures.

### Issue Context
The repo enables retries on CI (`retries: process.env.CI ? 2 : 0`). Worker-scoped fixtures do not get recreated for retries.

### Fix Focus Areas
Pick one of these approaches:
- Disable retries for suites using `@support/shared-page` (recommended if shared mutable state is intentional): add `test.describe.configure({ retries: 0 })` in the serial specs importing the shared fixture.
- Or change the fixture strategy to be retry-safe: make the context/page test-scoped (or re-created per test attempt) and preserve login via `storageState`/API login rather than a shared page.

- e2e-tests/playwright/support/shared-page.ts[34-75]
- e2e-tests/playwright.config.ts[53-61]
- e2e-tests/playwright/e2e/plugins/adoption-insights/adoption-insights.spec.ts[1-36]
- e2e-tests/playwright/e2e/plugins/scorecard/scorecard.spec.ts[17-47]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

With a shared worker-scoped BrowserContext, authentication state can leak across suites/files and
make subsequent login flows behave differently. Common.logintoKeycloak waits on a 'popup' event
without a timeout, so if the popup doesn't appear due to existing session state, the suite can hang
indefinitely.

Code

e2e-tests/playwright/support/shared-page.ts[R65-74]

+    { scope: "worker" },
+  ],
+
+  sharedPage: [
+    async ({ sharedContext }, use) => {
+      const page = await sharedContext.newPage();
+      await use(page);
+    },
+    { scope: "worker" },
+  ],

Relevance

⭐ Low

Speculative: Keycloak flow/popup behavior unclear; team previously rejected popup-flow robustness
suggestions, implying low appetite.

PR-#3797
PR-#4549
PR-#4315

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new fixture keeps a single BrowserContext alive for the whole worker (scope: "worker"). The
Keycloak login helper resolves only when a popup event fires; if the session is already
authenticated (possible when reusing context across suites), the popup may not open and the Promise
never resolves.

e2e-tests/playwright/support/shared-page.ts[34-75]
e2e-tests/playwright/utils/common.ts[99-128]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Common.logintoKeycloak()` awaits a Promise that only resolves on a `page.once('popup')` event. If the popup never opens (e.g., because the shared worker context is already logged in), the test run can hang.

### Issue Context
The new `@support/shared-page` fixture is worker-scoped, so auth/session state can persist beyond a single suite.

### Fix Focus Areas
- Add a bounded timeout around waiting for the popup event and fail fast with a clear error.
- Or make `loginAsKeycloakUser()` detect already-authenticated state (e.g., sidebar already visible / sign-in button absent) and return early.
- Optionally, add an explicit logout/state reset in suites using `sharedPage`.

- e2e-tests/playwright/utils/common.ts[99-128]
- e2e-tests/playwright/support/shared-page.ts[34-75]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

• Introduced worker-scoped sharedPage fixture for persistent browser context
• Integrated video recording, tracing, and screenshot support automatically
• Refactored adoption-insights and scorecard specs to use shared fixture
• Added @support/* TypeScript path alias for cleaner imports

New shared-page fixture with video and tracing

• Created new worker-scoped fixture providing persistent BrowserContext and Page
• Implemented video recording with 1280x720 resolution and automatic cleanup on success
• Added per-test tracing with startChunk/stopChunk and trace attachment to reports
• Tracks worker failures to retain video artifacts on test failures

e2e-tests/playwright/support/shared-page.ts

Migrate to shared-page fixture

• Changed import from @playwright/test to @support/shared-page
• Replaced manual browser.newContext() with sharedPage fixture parameter
• Removed manual context cleanup in test.afterAll()
• Removed unused context variable declaration

e2e-tests/playwright/e2e/plugins/adoption-insights/adoption-insights.spec.ts

Migrate to shared-page fixture

• Changed import from @playwright/test to @support/shared-page
• Removed BrowserContext type import, kept only Page type
• Replaced manual browser.newContext() with sharedPage fixture parameter
• Removed manual context cleanup in test.afterAll() and context variable

e2e-tests/playwright/e2e/plugins/scorecard/scorecard.spec.ts

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.